wasm_cc_binary: Use WASI OS for standalone builds#1262
wasm_cc_binary: Use WASI OS for standalone builds#1262walkingeyerobot merged 2 commits intoemscripten-core:mainfrom
Conversation
Problem: proxy-wasm/proxy-wasm-cpp-sdk#157 (comment) This is solving the problem in two different ways. Please let me know your thoughts about both approaches, as either will work. Signed-off-by: Martijn Stevenson <mstevenson@google.com>
walkingeyerobot
left a comment
There was a problem hiding this comment.
There's definitely larger amounts of work that could be done here to support wasi, but I think this change can be unobtrusive enough that we can accept this in some form.
| name = "platform_wasm", | ||
| constraint_values = [ | ||
| "@platforms//cpu:wasm32", | ||
| "@platforms//os:wasi", |
There was a problem hiding this comment.
definitely against this. the os is more appropriately emscripten, although that doesn't exist in bazel's upstream os list.
There was a problem hiding this comment.
ACK, added as platform_wasi instead
| default = False, | ||
| ), | ||
| "platform": attr.label( | ||
| default = "@emsdk//:platform_wasm", |
There was a problem hiding this comment.
instead of giving the user the ability to input an arbitrary platform string, what if we always set the platform to "emsdk//:platform_wasi" if attr.standalone == True and defaulted to "@emsdk//:platform_wasm" otherwise? if you want wasi, you're always going to set standalone. I don't know that the reverse is true, but I think it is, and we can try it and see if anyone complains (I suspect they will not).
There was a problem hiding this comment.
Sounds good, toggled off of attr.standalone.
Signed-off-by: Martijn Stevenson <mstevenson@google.com>
walkingeyerobot
left a comment
There was a problem hiding this comment.
Thanks very much!
…m. (emscripten-core#1262) * wasm_cc_binary: Specify a default OS. Allow users to override platform. Problem: proxy-wasm/proxy-wasm-cpp-sdk#157 (comment) This is solving the problem in two different ways. Please let me know your thoughts about both approaches, as either will work. Signed-off-by: Martijn Stevenson <mstevenson@google.com> * Rework platform selection to trigger os:wasi off standalone attr Signed-off-by: Martijn Stevenson <mstevenson@google.com> --------- Signed-off-by: Martijn Stevenson <mstevenson@google.com>
…m. (emscripten-core#1262) * wasm_cc_binary: Specify a default OS. Allow users to override platform. Problem: proxy-wasm/proxy-wasm-cpp-sdk#157 (comment) This is solving the problem in two different ways. Please let me know your thoughts about both approaches, as either will work. Signed-off-by: Martijn Stevenson <mstevenson@google.com> * Rework platform selection to trigger os:wasi off standalone attr Signed-off-by: Martijn Stevenson <mstevenson@google.com> --------- Signed-off-by: Martijn Stevenson <mstevenson@google.com>
Problem: proxy-wasm/proxy-wasm-cpp-sdk#157 (comment)
After discussion, we decided to target
@platforms//os:wasibased on attr.standalone.